-
-
Notifications
You must be signed in to change notification settings - Fork 475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(markdown): support markdown grammar code generation #3775
Conversation
CodSpeed Performance ReportMerging #3775 will degrade performances by 6.57%Comparing Summary
Benchmarks breakdown
|
ec0b39e
to
8d97371
Compare
I'm not sure what's your plan, but if your plan is to ship everything with one PR, please refrain from doing so:
Please consider breaking down the works in multiple PRs. Pros:
|
At this stage, the plan is like this
Then we start parsing the syntax
At present, the first two have been basically completed, so I will first modify this pr to achieve the first two goals. In the subsequent work, I will create an issue for subdivision, and then associate it through multiple pr. Is that OK |
Can someone review it for me? @ematipico @Conaclos |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: consider maybe using the prefix Md
instead of Markdown
for these syntax nodes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's a good idea, is it ok if I change it in the next pr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
Summary
grammer support
support commonmark grammer defined
Test Plan
add quick test